Add compile-time crypto callback async poll for record ciphers#10990
Add compile-time crypto callback async poll for record ciphers#10990julek-wolfssl wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a compile-time “poll to fill output” completion model for crypto-callback–backed async TLS record ciphers (AES + 3DES) via WOLF_CRYPTO_CB_ASYNC_POLL, aiming to prevent silent record corruption when a callback returns WC_PENDING_E by ensuring completion happens during async polling (or fails loudly when no completion path exists).
Changes:
- Adds a new crypto-callback algo type (
WC_ALGO_TYPE_ASYNC_POLL) andwc_CryptoCb_Poll()entry point to support poll-based completion. - Routes selected async operations through callback polling during
wolfSSL_AsyncPoll()/ async event polling, and converts “pending but no completion path” intoASYNC_OP_Efor TLS record ciphers. - Adds a dedicated async test suite (
tests/api/test_async.c) plus CI coverage to exercise poll completion and negative paths.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| wolfssl/wolfcrypt/types.h | Adds WC_ALGO_TYPE_ASYNC_POLL to identify callback poll re-entry. |
| wolfssl/wolfcrypt/settings.h | Adds a config-time guard for WOLF_CRYPTO_CB_ASYNC_POLL vs. WC_ASYNC_NO_CRYPT. |
| wolfssl/wolfcrypt/cryptocb.h | Declares wc_CryptoCb_Poll() for callback poll completion. |
| wolfssl/wolfcrypt/async.h | Extends WC_ASYNC_DEV to track a callback device ID for polling. |
| wolfcrypt/src/cryptocb.c | Implements wc_CryptoCb_Poll() to re-enter the callback with WC_ALGO_TYPE_ASYNC_POLL. |
| wolfcrypt/src/async.c | Integrates callback polling into async event polling/queue polling and sets routing state. |
| tests/api/test_async.h | Declares new async poll-completion unit tests and registration macro. |
| tests/api/test_async.c | Adds unit + TLS 1.3 integration tests for poll completion and negative cases. |
| tests/api/include.am | Adds new async test sources to the unit test build and dist list. |
| tests/api.c | Registers the new “async” test group in the test runner. |
| src/tls13.c | Fails fast with ASYNC_OP_E when a bulk cipher pends but no completion path exists. |
| src/internal.c | Applies the same “no completion path” failure for TLS 1.2 record ciphers; adjusts async pop logic for poll-completed events; preserves ssl->error on send failures. |
| examples/async/user_settings.h | Silences the new async-crypt + cryptocb TLS warning for the async example config. |
| configure.ac | Adds a configure-time warning about cryptocb+async TLS limitations and silences source-level warnings. |
| .wolfssl_known_macro_extras | Registers WOLF_CRYPTO_CB_ASYNC_POLL as a recognized macro. |
| .github/workflows/async-examples.yml | Adds CI job to build with WOLF_CRYPTO_CB_ASYNC_POLL and run the async test group. |
Comments suppressed due to low confidence (1)
wolfcrypt/src/async.c:772
- In wolfAsync_EventQueuePoll(), the callback poll is performed whenever cryptocb.devId is set, regardless of the event’s current ret value. Polling a non-pending event can incorrectly overwrite a successful completion with WC_NO_PENDING_E (or another error) depending on the device’s behavior. This should only poll when event->ret is still WC_PENDING_E.
if (asyncDev->cryptocb.devId != INVALID_DEVID) {
/* Re-enter the crypto callback to complete the job
* and fill the output buffer. */
event->ret =
wc_CryptoCb_Poll(asyncDev->cryptocb.devId);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
wolfcrypt/src/cryptocb.c:481
- wc_CryptoCb_Poll() currently lets WC_NO_PENDING_E propagate from the device callback. That contradicts the intent to never report “no pending” for an in-flight job and can also leave a DONE event with ret==WC_NO_PENDING_E, which wolfSSL_AsyncPop() treats specially (doesn’t clear ssl->asyncDev), potentially leaving stale async state. Consider mapping WC_NO_PENDING_E to a hard failure (WC_HW_E) like other unsupported polling cases.
ret = dev->cb(devId, &info, dev->ctx);
if (ret == WC_NO_ERR_TRACE(CRYPTOCB_UNAVAILABLE) ||
ret == WC_NO_ERR_TRACE(NOT_COMPILED_IN)) {
/* Device does not implement polling: job can never complete. */
ret = WC_NO_ERR_TRACE(WC_HW_E);
}
wolfcrypt/src/async.c:433
- wolfAsync_DevCtxInit() stamps asyncDev->cryptocb.devId for AES/3DES whenever WOLF_CRYPTO_CB_ASYNC_POLL is enabled. In builds that also enable a hardware async backend (Intel QAT / Cavium / async SW), this can cause wolfAsync_EventPoll() to attempt wc_CryptoCb_Poll() for a non-crypto-callback devId, overriding the real backend’s pending state and potentially forcing WC_HW_E. Consider only enabling the poll-routing stamp when crypto callbacks are the active async backend for bulk ciphers (i.e., not HAVE_INTEL_QA/HAVE_CAVIUM/WOLFSSL_ASYNC_CRYPT_SW).
#if defined(WOLF_CRYPTO_CB) && defined(WOLF_CRYPTO_CB_ASYNC_POLL)
A crypto callback that returns WC_PENDING_E for a TLS record cipher silently corrupted records: Encrypt()/Decrypt() advance the cipher state to CIPHER_STATE_END before the pending check, so on resume the record is shipped without re-running the cipher. WOLF_CRYPTO_CB_ASYNC_POLL gives crypto callback devices the QAT/Nitrox "poll to fill output" completion model. On WC_PENDING_E the async event stays queued; wolfSSL_AsyncPoll() re-enters the device with the new WC_ALGO_TYPE_ASYNC_POLL (wc_CryptoCb_Poll) to finish the job and fill the output buffer. The re-entry only polls while the event is still pending, and a device that cannot complete the job (no poll support, or it reports nothing pending) hard-fails with WC_HW_E rather than reporting the op done with an unfilled buffer. Only the two async record ciphers (AES and 3DES markers) are routed to poll completion, and only when crypto callbacks are the async backend (not QAT/Cavium/SW); handshake PK keeps the re-invoke model. The wolfSSL_AsyncPop eviction is gated for poll-capable devices so the existing resume-at-CIPHER_STATE_END path becomes correct with no record state-machine changes. Without the feature (and without a software/QAT/Cavium backend) a pending bulk cipher op now errors out with ASYNC_OP_E instead of corrupting the record, and configure/cryptocb.c warn about the unsupported combination. Tests in tests/api/test_async.c cover direct AES-GCM/CBC/CCM and 3DES poll completion in both directions at multiple pend depths, negative cases for cipher types defined but not dispatched (ChaCha, single DES), and full TLS 1.3 handshake+echo: an encrypt-offload run, a both- directions run that offloads encrypt and decrypt on both peers using a per-peer device, and the no-poll failure path.
9c754fc to
aab3257
Compare
wc_CryptoCb_Poll resolved the poll device with wc_CryptoCb_GetDevice, bypassing the WOLF_CRYPTO_CB_FIND remapping the cipher submit uses. With a device-find callback, submit could route to a mapped device while poll looked up the unmapped id and hard-failed with WC_HW_E. Resolve via wc_CryptoCb_FindDevice under WC_ALGO_TYPE_CIPHER so poll lands on the same device as submit.
| /* Resolve with WC_ALGO_TYPE_CIPHER, the algo the op was submitted under, so | ||
| * a WOLF_CRYPTO_CB_FIND remap lands on the same device as the submit. */ | ||
| CryptoCb* dev = wc_CryptoCb_FindDevice(devId, WC_ALGO_TYPE_CIPHER); | ||
| if (dev != NULL && dev->cb != NULL) { | ||
| wc_CryptoInfo info; | ||
| XMEMSET(&info, 0, sizeof(info)); | ||
| info.algo_type = WC_ALGO_TYPE_ASYNC_POLL; | ||
| ret = dev->cb(devId, &info, dev->ctx); | ||
| if (ret == WC_NO_ERR_TRACE(CRYPTOCB_UNAVAILABLE) || |
Summary
WOLF_CRYPTO_CB_ASYNC_POLL, giving crypto callback devices a QAT/Nitrox-style "poll to fill output" completion model for TLS record ciphers (AES and 3DES) instead of the re-invoke model, fixing silent record corruption when a callback returnsWC_PENDING_E.ASYNC_OP_Einstead of corrupting the record.Test plan
tests/api/test_async.ccovers direct AES-GCM/CBC/CCM and 3DES poll completion at multiple pend depths